Add QR-Code for Share Links #2162
Conversation
Himmelxd
force-pushed
the
himmel/qrcode-sharelinks
branch
from
8aa9ca3 to
125b678
Compare
|
@Himmelxd thanks for this PR :) I added some comments on the code |
Chartman123
added
enhancement
Himmelxd
force-pushed
the
himmel/qrcode-sharelinks
branch
from
ec92802 to
72ebf01
Compare
There was a problem hiding this comment.
Some more comments and in my opinion improvements. @susnux Please also have a look at it 🙂
Himmelxd
force-pushed
the
himmel/qrcode-sharelinks
branch
from
4936912 to
acc8f51
Compare
Himmelxd
force-pushed
the
himmel/qrcode-sharelinks
branch
3 times, most recently
from
9d3afde to
e71359c
Compare
Chartman123
added
3. to review
|
Thank you for your review and great comments :D |
Himmelxd
force-pushed
the
himmel/qrcode-sharelinks
branch
2 times, most recently
from
9794111 to
41ab25d
Compare
Himmelxd
force-pushed
the
himmel/qrcode-sharelinks
branch
from
d5ad0b1 to
8a4a227
Compare
Himmelxd
force-pushed
the
himmel/qrcode-sharelinks
branch
2 times, most recently
from
5137239 to
2888875
Compare
There was a problem hiding this comment.
Thank you for the changes!
But I still have some changes 🙈
I meant to abstract the whole dialog as a component, so also move the NcDialog to the QRDialog component.
Some minor stuff like making it reactive (currently in your version the uri is only built on create).
|
PS: My code suggestions are only to show what I meant, they are not tested and need some refactoring ;) |
Himmelxd
force-pushed
the
himmel/qrcode-sharelinks
branch
2 times, most recently
from
ddcb904 to
dd1351a
Compare
Himmelxd
force-pushed
the
himmel/qrcode-sharelinks
branch
5 times, most recently
from
b57a36d to
87b7b30
Compare
Himmelxd
force-pushed
the
himmel/qrcode-sharelinks
branch
from
87b7b30 to
a76bf27
Compare
|
@Himmelxd to fix the remaining Lint/Prettier errors, you should run Depending on your editor, there are also some extensions that allow formatting single files directly. |
Himmelxd
force-pushed
the
himmel/qrcode-sharelinks
branch
2 times, most recently
from
d207842 to
e65c830
Compare
There was a problem hiding this comment.
Everything's fine from my side :)
@susnux can we merge this now?
Himmelxd
force-pushed
the
himmel/qrcode-sharelinks
branch
from
e65c830 to
a5eb28d
Compare
Signed-off-by: Felix Beichler <[email protected]> resolve first comments Signed-off-by: Felix Beichler <[email protected]> move modal into tab vue Signed-off-by: Felix Beichler <[email protected]> remove spaces (again) Signed-off-by: Felix Beichler <[email protected]> refactor qr-variables to single object on this. Signed-off-by: Felix Beichler <[email protected]> resolve comments Signed-off-by: Felix Beichler <[email protected]> remove url label Signed-off-by: Felix Beichler <[email protected]> resolve comments Signed-off-by: Felix Beichler <[email protected]> parameterize alt text Signed-off-by: Felix Beichler <[email protected]> fix order of iconqr import Signed-off-by: Felix Beichler <[email protected]> change nc modal to dialog Signed-off-by: Felix Beichler <[email protected]> resolve Share {formTitle} Signed-off-by: Felix Beichler <[email protected]> separate qr dialog to component Signed-off-by: Felix Beichler <[email protected]> resolve comments Signed-off-by: Felix Beichler <[email protected]> Move dialog into component Signed-off-by: Felix Beichler <[email protected]> fix package-lock Signed-off-by: Felix Beichler <[email protected]> add copyright/license section fix lint Signed-off-by: Felix Beichler <[email protected]> fix package.json
Himmelxd
force-pushed
the
himmel/qrcode-sharelinks
branch
from
a5eb28d to
fa6ecdb
Compare
|
@Himmelxd Thank you very much, awesome work 🚀 Hope to see more contributions in the future! |
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Closes #1606
at least supposed to